Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: use theme colors in Banner #2932

Merged
merged 2 commits into from
Oct 19, 2021
Merged

feat: use theme colors in Banner #2932

merged 2 commits into from
Oct 19, 2021

Conversation

haxonadora
Copy link
Member

Summary

Closes #2899

Currently theme prop passed to Banner is only used for scale property. Users can't change banner colors such as surface, primary or text by providing custom theme prop. This PR uses colors provided by theme props to style Banner inner components

<Banner
  actions={[{ label: 'Fix it', onPress: () => {} }]}
  icon={require('../../assets/images/email-icon.png')}
  visible
  theme={{
    colors: {
      text: '#fff',  // text inside banner
      surface: '#09c8e5', // background
      primary: '#4e0ffc',  // button color
    },
  }}
>
  Lorem ipsum dolor sit amet, consectetur adipiscing elit
</Banner>

Screenshot 2021-10-15 at 23 33 30

Test plan

  1. Check if banner uses default theme colors when custom theme prop is not provided
  2. Check if providing custom theme to banner will change colors (surface, primary, text)

@haxonadora
feat: use theme colors in Banner
e1956b9 
fix: remove redundant array
@github-actions
Copy link

The mobile version of example app from this branch is ready! You can see it here

.

@callstack-bot
Copy link

callstack-bot commented Oct 15, 2021 •
edited
Loading

Hey @haxonadora, thank you for your pull request 🤗. The documentation from this branch can be viewed here.

@github-actions
Copy link

The mobile version of example app from this branch is ready! You can see it here

.

@lukewalczak
Copy link
Member

Hey @haxonadora thanks for the PR! Could you please replace one of the button in Banner example in favor of the button with the title SET CUSTOM THEME, which after being pressed, set custom colors like you presented on the screenshot

@haxonadora
Copy link
Member Author

Great idea @lukewalczak. I changed primary color a little bit so its easier to distinguish from default theme and added label swap for both themes
Screenshot 2021-10-16 at 22 11 33
Screenshot 2021-10-16 at 22 11 41

@github-actions
Copy link

The mobile version of example app from this branch is ready! You can see it here

.

lukewalczak
lukewalczak approved these changes Oct 19, 2021
View reviewed changes
Copy link
Member

@lukewalczak lukewalczak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @haxonadora for your work!

@lukewalczak lukewalczak merged commit ee9c2d0 into main Oct 19, 2021
@lukewalczak lukewalczak deleted the 2899 branch October 19, 2021 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lukewalczak lukewalczak lukewalczak approved these changes

Assignees
No one assigned
Labels
Banner Hacktoberfest Theme
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

<Banner theme={specificComponentTheme}> is not working
3 participants
@haxonadora @callstack-bot @lukewalczak